Repository navigation
perf(qwp): improved Gorilla timestamp decoding - #69
glasstiger wants to merge 7 commits into
Conversation
decode() and decodeView() each held a copy of a Gorilla loop that read the bitstream one bit at a time and built every delta-of-delta from per-bit BigInt operations. Both now call one decoder that reads through a 32-bit bit buffer, carries the int64 delta and timestamp as int32 halves with explicit carries, wrapping exactly as BigInt.asIntN(64, ...) does, and emits runs of repeated deltas in one step. Values are stored as int32 word pairs through an Int32Array, with a byte swap on big-endian hosts so the bytes stay little-endian. Decoded values and errors are unchanged. A stream too short to hold one bit per row is now rejected before any storage is sized for it, and decode() reuses one decode buffer per decoder across batches, as column views already did. On 10,000-row batches in Node 24, column views decode 3.2-32x faster and materialized batches 1.8-27x faster, least for a constant interval and most for irregular timestamps. JavaScriptCore views decode 66-107x faster. Adds code-width, NULL, int64-edge, and decoder-reuse tests for both paths, a 600-stream comparison against a bit-at-a-time BigInt reference decoder, every malformed-column error, and timestamp-only Gorilla arms in benchmarks/egress.bench.ts.
The code-width test reused one decoder only with shorter batches, so nothing exercised growing the retained Gorilla storage: a decoder that stopped growing it passed every test while a larger later batch threw a RangeError. A three-value batch now precedes the main frame, which has to grow that storage in both decode() and decodeView(). The test's `max` column claimed to cover the top of the int64 range but peaked about 7.1e11 below INT64_MAX. It now ends exactly at INT64_MAX.
The Gorilla decoder writes int32 words through an Int32Array and byte-swaps them on big-endian hosts, so column views still expose little-endian int64 bytes. No test reached that branch: NATIVE_LITTLE_ENDIAN is fixed when result-batch.ts loads and every CI host is little-endian, so deleting the swap call left all 897 QWP tests passing. The new test makes the module's Uint16Array.of(1) probe see big-endian storage, loads a fresh copy, and decodes through decode() and decodeView(). On a little-endian host the swap then shows as byte-reversed int32 halves in every value. It fails if the swap call is deleted or inverted, or if the swap drops a byte, shifts one wrong, or covers only half the words.
The decoder's comment said the speedup was least for a constant interval and most for irregular timestamps, in V8 and JavaScriptCore alike. In JavaScriptCore the jittered series gains least: two Bun 1.0.14 runs measured 66x and 61.7x for it, against 72x and 72.5x for a constant interval. The ordering now applies to V8 only, and the JavaScriptCore figure reads "over 60x" rather than a 65-110x range that the second run fell below.
|
Level 3 review — approve (head No admitted findings: Critical 0, Moderate 0, Minor 0; in-diff 0, out-of-diff breakage 0. Test gate passes with 0 admitted coverage gaps. The full suite passed (1,127 tests), along with package-boundary and browser suites, typechecks, lint, and formatting checks. Base/head benchmarks reproduced the claimed Gorilla decoding speedup. The decoder retains row-cap-bounded scratch storage; no regression was observed. Submodules: none. |
|
Level 3 review — approve (head No findings in this PR: Critical 0, Moderate 0, Minor 0. The test gate passes with no coverage gaps worth reporting. Evidence
Tradeoff: Pre-existing bug (not from this PR, not blocking; worth its own issue)
const decoder = new QwpResultBatchDecoder();
decoder.decodeView(message, 1); // slot 0 never used
decoder.resetQuerySchema(); // TypeError: Cannot read properties of undefined (reading 'release')The schema and batch-sequence reset after the loop then never run. Reproduced on base and head (Node 24, Bun 1.0.14). Suggested fix: Submodules: none. |
glasstiger
changed the title
glasstiger
changed the title
glasstiger
changed the title
javier
left a comment
javier
left a comment
There was a problem hiding this comment.
Tested and I can confirm it sped up when the bottleneck was decoding (in my case, with a client running in the same VPC as the server on EC2)
The big-endian Gorilla test makes the module's Uint16Array.of(1) probe report big-endian storage, then expected the byte-reversed int32 halves that a little-endian host decodes. On a real big-endian host the swap is the actual conversion, so both decoders return the true values: the test failed on s390x Node 20, 22, and 26 under QEMU, and passed there only with the swap deleted. It now expects the swapped values on a little-endian host and the true values on a big-endian one, where a missing swap fails it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
decode() read every decoded Gorilla timestamp out of its int32 words with DataView.getBigInt64, which is slow in the V8 of Node 20 and 22: there, a constant-interval timestamp column decoded at 0.7-0.8x the speed of main for 1,000-10,000 rows. On little-endian hosts the words are now read through a BigInt64Array, which makes decode() 1.4-1.9x as fast as main on Node 20.18.1 and 22.17.0 and leaves Node 24 unchanged. Big-endian hosts keep the DataView, because a BigInt64Array reads native byte order. The big-endian test now also gives the simulated copy a big-endian BigInt64Array while it decodes. Without it, removing the little-endian check passed every test on a little-endian host; on s390x that change fails four Gorilla tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Level 3 review — approve (head No admitted findings: 0 Critical, 0 Moderate, 0 Minor; 0 in-diff, 0 out-of-diff breakage. The test gate passes with 0 admitted coverage gaps. Evidence
Tradeoff (stated in the PR): Pre-existing bug (not from this PR, not blocking; fixed in #71)
PR metadata: the title follows Conventional Commits and the description states the user impact. Submodules: none. |
This branch has not been deployed
2 participants
Summary
Gorilla-encoded
TIMESTAMP,TIMESTAMP_NANOS, andDATEcolumns in query results now decode without BigInt arithmetic, inpackages/client-core/src/_qwp/_core/result-batch.ts. In testing, a QuestDB nightly server flagged every result batch as Gorilla-encoded, so this path decodes the timestamp columns of typical query results in both@questdb/nodejs-clientand@questdb/browser-client. No public API change, and decoded values and errors are unchanged.decode()anddecodeView()each held a copy of a loop that read the bitstream one bit at a time and built every delta-of-delta from per-bit BigInt operations (value |= 1n << BigInt(bit)). Both now calldecodeGorillaTimestamps(), which reads through a 32-bit bit buffer, carries the int64 delta and timestamp as int32 halves with explicit carries (wrapping exactly asBigInt.asIntN(64, ...)does), and emits runs of repeated deltas in one step.Int32Array, which ran the decode loop 1.8-4.7x faster thanDataViewstores in V8. A byte-swap pass runs only on big-endian hosts.truncated QWP Gorilla bitstream).decode(). One buffer per decoder is reused across batches, as column views already did.BigInt64Arrayread-out. On little-endian hosts,decode()reads the values out of that buffer through aBigInt64Array. Read throughDataView.getBigInt64, materialized constant-interval batches ran at 0.72x (Node 20) and 0.76x (Node 22) the speed ofmain. Big-endian hosts keep theDataView, because aBigInt64Arrayreads native byte order, in a loop deliberately separate fromreadInt64Values(): with one loop serving both, V8 ran this path up to 2x slower, depending on which caller it optimized first.decode()anddecodeView(); 600 random streams (including truncated ones and int64 wraparound) against a bit-at-a-time BigInt reference decoder, comparing values, consumed bytes, and errors; every malformed-column error through both paths; a simulated big-endian host (the module's endianness probe andBigInt64Arrayreads made big-endian), which expects byte-swapped values on a little-endian host and the true values on a big-endian one.benchmarks/egress.bench.ts(constant, jittered, and irregular intervals).benchmarks/workloads.tsexports its existing xorshift generator for them.Performance
Diagnostic, not CI gates. Before is
main; two interleaved runs each, averaged.Node 24.3.0,
benchmarks/egress.bench.ts, one GorillaTIMESTAMPcolumn of 10k rows, batches/s:mainmainThe same arms on Node 20.18.1 and 22.17.0,
main→ this PR:The existing mixed batch (INT, DOUBLE, VARCHAR, constant-interval Gorilla TIMESTAMP): reusable column views 16,025 → 37,560 batches/s on Node 24 (2.3x; 4.1x on Node 20 and 5.2x on Node 22),
decode()materialized 881 → 926 (1.05x; 1.03x and 1.04x). The traversal arms decode the same batch, Gorilla column included, on every iteration: within 1% on Node 24 and 2-8% faster on Node 20 and 22. The Zstd arm does not exercise this code and is unchanged within noise.JavaScriptCore (Bun 1.0.14, Safari's engine), the built browser bundle, the same three interval shapes, 10k rows, median µs per batch:
mainmainThese JavaScriptCore figures predate the
BigInt64Arrayread-out, which changes only the materialized path: in a separate harness it made materialized decoding in Bun a further 1.03-1.09x faster.The gain is larger in JavaScriptCore because it allocates on every BigInt operation, while V8 lowers
BigInt.asIntN(64, ...)arithmetic to native int64 math. Materializing abigint[]still costs one BigInt allocation per value, which bounds the materialized speedup.Validation
pnpm exec vitest run(1,145 tests, including container integration),pnpm test:dist,pnpm test:qwp-browserpnpm typecheck,pnpm typecheck:test,pnpm typecheck:qwp-browser,pnpm typecheck:bench,pnpm typecheck:distpnpm eslint,pnpm lint:bench,pnpm format:check,pnpm check:packagesDataViewfallback, an off-by-one index). The new tests fail for each.TIMESTAMP,TIMESTAMP_NS, andDATE. Every batch was Gorilla-flagged; on Node 20.18.1, 22.17.0, and 24.3.0 this PR decoded the same values asmainthroughquery()andqueryViews(), and the first and last rows matched HTTP/exec.main: six Zstd tests and one test that exceeds its timeout under emulation.